Skip to content
This repository has been archived by the owner on Jan 23, 2023. It is now read-only.

Fix the package restore for arm64 and arm32 #12532

Merged
merged 1 commit into from
Jun 29, 2017

Conversation

jashook
Copy link

@jashook jashook commented Jun 28, 2017

/cc @sdmaclea I am expecting this to fix the Arm64 Windows jobs.

@jashook
Copy link
Author

jashook commented Jun 28, 2017

@dotnet-bot test Windows_NT Arm64

@jashook
Copy link
Author

jashook commented Jun 28, 2017

/cc @BruceForstall

@@ -31,7 +31,19 @@
<PropertyGroup>
</PropertyGroup>
</When>
<When Condition="'$(OSGroup)'=='Windows_NT'">
<When Condition="'$(OSGroup)'=='Windows_NT' AND '$(__BuildArch)'=='arm64'">
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you not use an OR here (arm64 OR arm)?

@wtgodbe
Copy link
Member

wtgodbe commented Jun 28, 2017

LGTM, other than the 1 comment. Guessing this got package restore working locally?

@jashook
Copy link
Author

jashook commented Jun 28, 2017

Correct managed to get the restore working locally. I will work on the OR change.

@sdmaclea
Copy link

@jashook Thanks

<TestNugetRuntimeId>win-$(__BuildArch)</TestNugetRuntimeId>
</PropertyGroup>
</When>
<When Condition="'$(OSGroup)'=='Windows_NT' AND '$(__BuildArch)'!='$(arm64)' AND '$(__BuildArch)'!='$(arm)'">
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this be:

<When Condition="'$(OSGroup)'=='Windows_NT' AND '$(__BuildArch)'!='arm64' AND '$(__BuildArch)'!='arm'">

?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes that is a typo thank you for catching that

@BruceForstall
Copy link
Member

@wtgodbe , others: is there a reason we can't just name the arm/arm64 packages (or whatever this is naming) using the same naming scheme as x86/x64?

@jashook jashook force-pushed the fix_arm64_and_arm32_package_restore branch from 966cd34 to 987c2fc Compare June 28, 2017 21:31
@jashook
Copy link
Author

jashook commented Jun 28, 2017

@dotnet-bot test Windows_NT Arm64

@wtgodbe
Copy link
Member

wtgodbe commented Jun 28, 2017

@BruceForstall we only publish portable packages now, which have the prefix win-{arch}. So actually, @jashook, as part of this change it might be good to just change all testNugetRuntimeID's to win-{arch} for Windows, as today in CI we're probably downloading old win7-{arch} packages for x64 & x86.

@sdmaclea
Copy link

@BruceForstall I think part of the problem is the absence of arm64 linux nuget packages. #12372 And dotnet/corefx#21236 should fix that.

@jashook
Copy link
Author

jashook commented Jun 28, 2017

@dotnet-bot test Windows_NT Arm64 Checked

1 similar comment
@jashook
Copy link
Author

jashook commented Jun 29, 2017

@dotnet-bot test Windows_NT Arm64 Checked

@jashook jashook force-pushed the fix_arm64_and_arm32_package_restore branch from 987c2fc to 0489dac Compare June 29, 2017 16:12
@jashook
Copy link
Author

jashook commented Jun 29, 2017

@dotnet-bot test Windows_NT Arm64 Checked

@jashook
Copy link
Author

jashook commented Jun 29, 2017

Updated ptal @wtgodbe

@wtgodbe
Copy link
Member

wtgodbe commented Jun 29, 2017

LGTM pending CI

@jashook jashook merged commit 07258b7 into dotnet:master Jun 29, 2017
@jashook jashook deleted the fix_arm64_and_arm32_package_restore branch June 29, 2017 19:25
@karelz karelz modified the milestone: 2.1.0 Aug 28, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants